Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add project compat entry viewing & editing #2702

Merged
merged 1 commit into from
Sep 21, 2021

Conversation

IanButterworth
Copy link
Sponsor Member

@IanButterworth IanButterworth commented Aug 15, 2021

Updated: Was -e|--edit and defaulted to the static version.

Adds a Project compat section viewer via pkg> compat and an interactive editor via pkg> compat -l|--list.

Also some API endpoints

  • Pkg.compat!(): equivalent to pkg> compat (errors early if run in a non-interactive terminal)
  • Pkg.compat(): equivalent to pkg> compat -l
  • Pkg.compat("Foo"): Return compat entry for Foo (nothing if no entry)
  • Pkg.compat!("Foo", "1.2"): Set entry for Foo (use nothing or "" to remove the entry)

I think there a few benefits to adding this:

  • compat entries can be edited without leaving the repl
  • it validates that new entries are valid compat version specifiers before they're written to file
  • could be expanded to do more stuff i.e.

Todo (if approved of)

  • Tests (the interactive editor isn't testable so the codecov diff misses)
  • Edit entry rather than retype. Implemented via raw stdin manipulation

Screen Shot 2021-08-15 at 12 04 40 AM

@johnnychen94
Copy link
Sponsor Member

johnnychen94 commented Aug 15, 2021

It would be easier to realize the existence of edit mode if there's one line description after/before pkg> compat output, e.g., "To interactively edit entries values, use pkg> compat -e" (maybe only print in interactive mode)

Another question: is the compat edit affecting immediately, or it saves onto some in-memory struct and only changes the "Project.toml" until you quit the edit mode? I'm thinking of the second one because it provides some rollback mechanism (sort of dry run) besides fewer IO operations.

@IanButterworth
Copy link
Sponsor Member Author

Yeah I wondered about adding a note on edit mode. It's not done for any other pkg> mode though, so it might feel OTT. I'll add one to see how it feels

Another question: is the compat edit affecting immediately, or it saves onto some in-memory struct and only changes the "Project.toml" until you quit the edit mode? I'm thinking of the second one because it provides some rollback mechanism (sort of dry run) besides fewer IO operations.

It acts immediately on the file but complies with the pkg> undo functionality, so everything can be rolled back or forward with pkg> redo. That's the standard practice across Pkg.

@IanButterworth
Copy link
Sponsor Member Author

I think this will get annoying quickly
Screenshot from 2021-08-15 16-46-18

Perhaps julia/pkg could have a tip mechanism that only tips the first time a function is used. There could be a dict stored on disk.. but that's a whole thing

@IanButterworth IanButterworth force-pushed the ib/compat_info branch 8 times, most recently from a0084d8 to 237105a Compare August 16, 2021 04:30
@StefanKarpinski
Copy link
Sponsor Member

This is very cool and useful. Let's punt on the tip thing for now. People can RTFM or be told about compat -e. Alternatively, make edit mode the default (since it does view if you do nothing) and have a different flag to tell the compat command just to dump the current compat entries.

@IanButterworth
Copy link
Sponsor Member Author

Yeah that sounds good. I've switched pkg> compat over to the interactive edit mode, and pkg> compat -l|--list for the static list.
Also dropped the tip

@IanButterworth
Copy link
Sponsor Member Author

Seems worth a go? My only remaining thought is whether the API occupies space that might be better used for something else?

@DilumAluthge
Copy link
Member

Personally, I can't think of another meaning for ] compat than either "show [compat] entries" or "edit [compat] entries".

@DilumAluthge
Copy link
Member

I know that @GunnarFarneback has built similar functionality in a package. He might be able to comment on any lessons or best practices that have been learned from the package and its users.

@DilumAluthge
Copy link
Member

DilumAluthge commented Aug 18, 2021

Also, there is at least one open issue requesting this feature, so I think that this pull request would close that issue.

@IanButterworth
Copy link
Sponsor Member Author

Also, there is at least one open issue requesting this feature, so I think that this pull request would close that issue.

Maybe #341 ?
#2463 is related

Otherwise I couldn't find anything else

@DilumAluthge
Copy link
Member

Yeah I think this closes #341.

@GunnarFarneback
Copy link
Contributor

I know that @GunnarFarneback has built similar functionality in a package. He might be able to comment on any lessons or best practices that have been learned from the package and its users.

Yes, PackageCompatUI.

The feedback can mostly be summarized as 50 likes on the discourse announcement, 23 stars on the Github repo and an appreciation on slack.

@IanButterworth
Copy link
Sponsor Member Author

Oh cool! The version selection through radio menu and automatic collapsing of version spec is really nice! It would've been nice if I'd found that sooner 😬

I'm not sure what to do now. Is there a reason you didn't consider developing that as part of Pkg @GunnarFarneback ?

Some thoughts if this PR goes forward:

  • The way this PR prints the info as part of the menu seems like a better fit for how Pkg displays info
  • Incorporating the version selection radio menu system could be good, but I think also just letting the user edit the text and have that validated might be ok as a default?
  • Highlighting when a compat entry doesn't include the latest version of the package is great, and that should be added

@KristofferC
Copy link
Sponsor Member

Interactivity is cool but for the specific case of adding compat, I don't really see that big of an advantage over just specifying the package and the compat string directly (or just using a normal text editor). Let's take rm as an example, you just give a list of package names to it. It doesn't provide a list where you interactively mark what packages to remove etc. With tab-completion such an interface seems a lot quicker than a menu-driven one (and it is easier to test etc)

Some other thoughts:

One question is if compat should be status --compat?

Highlighting when a compat entry doesn't include the latest version of the package is great, and that should be added

The status --outdated shows this as well so it should be fairly easy to use the implementation there.

@GunnarFarneback
Copy link
Contributor

The main reason for not doing it inside Pkg is that I wanted the feature to show the registration date of package versions, which seemed unreasonable to include in Pkg as it requires a git clone of the registry and is somewhat slow, plus more freedom to try things out.

Incorporating the version selection radio menu system could be good, but I think also just letting the user edit the text and have that validated might be ok as a default?

The primary advantage of the menu selection is that the user gets a correct compat entry without knowing the version specifier format. The main disadvantage is that some things can't really be expressed that way, e.g. compat with Julia 1.7 before it's released.

@IanButterworth
Copy link
Sponsor Member Author

IanButterworth commented Aug 18, 2021

Replying to @KristofferC
pkg> status --compat is a good suggestion. I'll do that.

I'll add the basic pkg> compat Foo 0.2,0.3,0.4 editing form too.

I still think there's a benefit to prompting the edit with the current string for manipulation, as it would be a lot to expect the user to retype/copy-paste the current entry in, to insert a new version. It may also also be error-prone.

Perhaps a form pkg> compat Foo that would launch the prompt directly would be worth adding too.

All of those additions could be CI tested. Come to think of it, so can the full menu, by pre-loading the stdin buffer before running the menu.

@IanButterworth
Copy link
Sponsor Member Author

Perhaps a form pkg> compat Foo that would launch the prompt directly would be worth adding too.

Tab completion! That's the way to do it probably. pkg> compat Foo<tab> completes the current entry for editing.

@StefanKarpinski
Copy link
Sponsor Member

I still think there's a benefit to prompting the edit with the current string for manipulation, as it would be a lot to expect the user to retype/copy-paste the current entry in, to insert a new version. It may also also be error-prone.

Strongly agree with this. I'd be happy to run this to capture the current versions as lower bounds but I'd be annoyed to have to look up what version each dep is on and add the bound manually.

@IanButterworth IanButterworth force-pushed the ib/compat_info branch 2 times, most recently from e16eec9 to 8fb1c8d Compare September 5, 2021 04:05
@IanButterworth
Copy link
Sponsor Member Author

Update
pkg> status --compat now prints the full compat entries, or Pkg.status(compat = true)
pkg> status --compat Foo Bar julia now prints the specific entries for Foo, Bar, julia

pkg> compat launches the interactive editor
pkg> compat Foo 0.8,0.9 now supports tab completion, so you can type pkg> compat Fo and it will complete to pkg> compat Foo 0.8,0.9 so you can edit the entry without retyping it
pkg> compat Foo will remove the entry, equivalent to Pkg.compat("Foo", nothing)

@IanButterworth
Copy link
Sponsor Member Author

Mergeable?

@IanButterworth
Copy link
Sponsor Member Author

I think there's support in this current form, so I'll merge this at the weekend if I don't hear otherwise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants